Skip to content

[Cropper] Always apply rotation in Crop::getCroppedImage() and Crop::getCroppedThumbnail()#3433

Merged
Kocal merged 1 commit intosymfony:3.xfrom
MrYamous:cropper-road-to-3x
Apr 4, 2026
Merged

[Cropper] Always apply rotation in Crop::getCroppedImage() and Crop::getCroppedThumbnail()#3433
Kocal merged 1 commit intosymfony:3.xfrom
MrYamous:cropper-road-to-3x

Conversation

@MrYamous
Copy link
Copy Markdown
Contributor

@MrYamous MrYamous commented Apr 4, 2026

Q A
Bug fix? no
New feature? no
Deprecations? no
Documentation? i'll check if needed
Issues Next part of #2930
License MIT

I saw some PR about 3.x and tests dedicated to this change were removed, but not yet the code

@carsonbot carsonbot added the Status: Needs Review Needs to be reviewed label Apr 4, 2026
Copy link
Copy Markdown
Member

@Kocal Kocal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, nice catch!

Shouldn't we always apply the rotation like the deprecation message told us? Meaning, we can remove the $applyRotation parameter

@Kocal Kocal added Status: Needs Work Additional work is needed Feature New Feature Cropperjs and removed Status: Needs Review Needs to be reviewed labels Apr 4, 2026
@carsonbot carsonbot changed the title [Cropper] change applyRotation default value to true [Cropperjs][Cropper] change applyRotation default value to true Apr 4, 2026
@MrYamous
Copy link
Copy Markdown
Contributor Author

MrYamous commented Apr 4, 2026

Oops, nice catch!

Shouldn't we always apply the rotation like the deprecation message told us? Meaning, we can remove the $applyRotation parameter

At first look, I didn't like the idea of implicit rotation based on whether or not related options were present.
But in the end, I figure that this parameter doesn't really add much.
I'm going to make the change

@carsonbot carsonbot added Status: Needs Review Needs to be reviewed and removed Status: Needs Work Additional work is needed labels Apr 4, 2026
@MrYamous MrYamous changed the title [Cropperjs][Cropper] change applyRotation default value to true [Cropperjs] remove applyRotation parameter Apr 4, 2026
@Kocal Kocal force-pushed the cropper-road-to-3x branch from adbf325 to 6c045e4 Compare April 4, 2026 12:42
@Kocal Kocal force-pushed the cropper-road-to-3x branch from 6c045e4 to 50060bf Compare April 4, 2026 12:43
@Kocal Kocal changed the title [Cropperjs] remove applyRotation parameter [Cropper] Always apply rotation in Crop::getCroppedImage() and Crop::getCroppedThumbnail() Apr 4, 2026
@Kocal
Copy link
Copy Markdown
Member

Kocal commented Apr 4, 2026

Thank you @MrYamous.

@Kocal Kocal merged commit 2dc151e into symfony:3.x Apr 4, 2026
24 of 27 checks passed
@MrYamous MrYamous deleted the cropper-road-to-3x branch April 4, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Cropperjs Feature New Feature Status: Needs Review Needs to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants